[MOB-1199] dev: update index constraints for notifications to optimize query performance #7870
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughDatabase indexes are added to the Notification and FileAsset models to optimize query performance on receiver/workspace lookups and entity identifier searches. A non-atomic Django migration implements these six indexes using concurrent creation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
…otification-fields
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes query performance for the notification table by adding strategic database indexes to support common query patterns.
- Added 5 new composite indexes to the notification table
- Created Django migration to implement the index changes
- Focused on receiver-based queries, entity lookups, and notification state filtering
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/api/plane/db/models/notification.py | Added 5 new composite indexes to optimize notification queries |
| apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py | Django migration to create the new indexes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py (1)
13-32: Consider concurrent index creation for production deployments.The migration uses standard
AddIndexoperations, which will lock the table during index creation. For a largenotificationstable in production, this could cause downtime or blocking.Django supports concurrent index creation via
CREATE INDEX CONCURRENTLY(PostgreSQL). Consider modifying this migration to usemigrations.AddIndexConcurrentlyif deploying to a production database with significant data volume.Example modification for concurrent index creation:
from django.contrib.postgres.operations import AddIndexConcurrently class Migration(migrations.Migration): atomic = False # Required for concurrent operations operations = [ AddIndexConcurrently( model_name='notification', index=models.Index(fields=['receiver', 'workspace', 'read_at', 'created_at'], name='notif_receiver_status_idx'), ), # ... repeat for other indexes ]Note: This requires PostgreSQL and adds complexity to rollback scenarios.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py(1 hunks)apps/api/plane/db/models/notification.py(1 hunks)
🔇 Additional comments (5)
apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py (2)
1-33: Migration matches model definitions correctly.The migration operations correctly mirror the index definitions in the model file. All five indexes have matching field orders and names.
8-10: Dependency chain verified. Migration 0107 exists and directly precedes 0108—no conflicts detected.apps/api/plane/db/models/notification.py (3)
50-52: Index usage confirmed. Multiple ORM queries filter onsnoozed_tillandarchived_at(e.g., in apps/api/plane/app/views/notification/base.py around lines 72–87), so the composite index onreceiver, workspace, snoozed_till, archived_atis appropriately utilized.
57-60: Unable to locate usage of composite key filters
I couldn’t find any code filtering notifications byworkspace,entity_identifier, andentity_nametogether. Please confirm such queries exist to justify the composite index or consider removing it.
41-44: Confirm index field order matches actual query filters.
No occurrences of queries filtering by bothreceiverandworkspacewere found—please manually verify that["receiver", "workspace", "read_at", "created_at"]aligns with your most common WHERE clauses and sort patterns.
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on October 20. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py (1)
14-35: Consider index maintenance overhead and monitoring.Adding five composite indexes will improve read performance but increases:
- Write operation overhead (INSERT/UPDATE/DELETE on notifications)
- Storage requirements
- Index maintenance during vacuuming
Recommendations:
- Monitor index usage after deployment using PostgreSQL's
pg_stat_user_indexesview to identify unused indexes- Track index bloat and schedule periodic REINDEX operations if needed
- Measure query performance before/after to quantify improvements
- Consider index-only scans by including frequently-selected columns in indexes if query patterns support it
Example monitoring query:
SELECT schemaname, tablename, indexname, idx_scan, idx_tup_read, idx_tup_fetch FROM pg_stat_user_indexes WHERE tablename = 'notification' ORDER BY idx_scan;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py (2)
1-12: LGTM! Proper setup for concurrent index creation.The migration correctly imports
AddIndexConcurrentlyand setsatomic = False, which is required for concurrent index operations in PostgreSQL to avoid table locking during index creation.
14-35: Composite indexes align with model schema and expected query patterns.All five indexes use non‐nullable leading columns (
receiver,workspace), nullable fields (read_at,snoozed_till,archived_at) appear later, and no redundant coverage exists given existing single‐ and two‐column indexes. Migrations match theMeta.indexesinNotification. No changes needed.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/plane/db/migrations/0111_notification_notif_receiver_status_idx_and_more.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
| # Generated by Django 4.2.22 on 2025-09-29 15:36 | ||
|
|
||
| from django.db import migrations, models | ||
| from django.contrib.postgres.operations import AddIndexConcurrently | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
| atomic = False | ||
|
|
||
| dependencies = [ | ||
| ('db', '0110_workspaceuserproperties_navigation_control_preference_and_more'), | ||
| ] |
There was a problem hiding this comment.
Migration number discrepancy with PR objectives.
The PR description references migration 0108_*, but this file is numbered 0111_*. This suggests either the PR description is outdated or there were additional migrations created between the initial plan and implementation.
Good use of atomic = False with AddIndexConcurrently to prevent table locking during index creation.
| operations = [ | ||
| AddIndexConcurrently( | ||
| model_name='notification', | ||
| index=models.Index(fields=['receiver', 'workspace', 'read_at', 'created_at'], name='notif_receiver_status_idx'), | ||
| ), | ||
| AddIndexConcurrently( | ||
| model_name='notification', | ||
| index=models.Index(fields=['receiver', 'workspace', 'entity_name', 'read_at'], name='notif_receiver_entity_idx'), | ||
| ), | ||
| AddIndexConcurrently( | ||
| model_name='notification', | ||
| index=models.Index(fields=['receiver', 'workspace', 'snoozed_till', 'archived_at'], name='notif_receiver_state_idx'), | ||
| ), | ||
| AddIndexConcurrently( | ||
| model_name='notification', | ||
| index=models.Index(fields=['receiver', 'workspace', 'sender'], name='notif_receiver_sender_idx'), | ||
| ), | ||
| AddIndexConcurrently( | ||
| model_name='notification', | ||
| index=models.Index(fields=['workspace', 'entity_identifier', 'entity_name'], name='notif_entity_lookup_idx'), | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
Verify composite index field order matches query patterns.
The field order in composite indexes is critical for performance due to the leftmost prefix rule. Ensure that the index field orders align with your most common query patterns:
notif_receiver_status_idx: Queries should filter byreceiver(and optionallyworkspace,read_at,created_at) in that ordernotif_receiver_entity_idx: Queries should filter byreceiver(and optionallyworkspace,entity_name,read_at) in that order- Similar verification needed for the other three notification indexes
Also verify that these new indexes don't significantly overlap with existing indexes, which could waste storage and slow down writes.
Run the following script to check for existing indexes on the notification table:
🏁 Script executed:
#!/bin/bash
# Description: Check for existing indexes on notification model that might overlap
# Search for existing indexes in the Notification model Meta class
ast-grep --pattern $'class Notification($$$):
$$$
class Meta:
$$$
indexes = $$$
$$$'
# Also search for any index-related constraints or db_index fields
rg -nP --type=py -C5 'class Notification|db_index\s*=\s*True' -g '**/models/*notification*.py'Length of output: 917
I need to gather more complete information about the Notification model and how it's queried. Let me retrieve the full model definition and search for query patterns.
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the complete Notification model file
cat apps/api/plane/db/models/notification.py
Length of output: 5145
🏁 Script executed:
#!/bin/bash
# Search for query patterns on the notification model to verify field order alignment
rg -nP --type=py 'Notification\.objects\.(filter|exclude|get)' -C3 -g '!migrations' | head -100Length of output: 6211
🏁 Script executed:
#!/bin/bash
# Check for any existing indexes or constraints on notification fields
rg -nP --type=py 'class.*Notification.*Meta|indexes\s*=|db_index' apps/api/plane/db/models/notification.pyLength of output: 22
🏁 Script executed:
#!/bin/bash
# Look for existing index definitions in all migrations related to notification
fd '.*notification.*\.py' apps/api/plane/db/migrations/ | xargs grep -l 'AddIndex\|RemoveIndex' | head -20Length of output: 84
🏁 Script executed:
#!/bin/bash
# Check the actual index creation/removal statements in older migrations
rg -nP --type=py 'AddIndex|RemoveIndex' apps/api/plane/db/migrations/ -B2 -A5 | grep -i notification | head -60Length of output: 4912
I need to verify whether these indexes are redundantly defined and check migration dependencies.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check all migrations to find when model indexes were added
rg -n 'notif_receiver_status_idx|notif_entity_idx|notif_receiver_entity_idx|notif_receiver_state_idx|notif_receiver_sender_idx|notif_entity_lookup_idx' apps/api/plane/db/migrations/
Length of output: 1222
🏁 Script executed:
#!/bin/bash
# Check the complete migration file 0111 to see dependencies
cat apps/api/plane/db/migrations/0111_notification_notif_receiver_status_idx_and_more.pyLength of output: 1523
🏁 Script executed:
#!/bin/bash
# Search for earlier migrations that might have added these indexes
fd '010[0-9].*\.py' apps/api/plane/db/migrations/ | sort | xargs grep -l 'notif_receiver\|notif_entity'Length of output: 77
🏁 Script executed:
#!/bin/bash
# Check if the model Meta.indexes were recently added or always present
git log --oneline --all -- apps/api/plane/db/models/notification.py 2>/dev/null | head -20 || echo "Git history not available"Length of output: 280
Let me check the git commits to understand the sequence of changes:
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the specific commit that updated index constraints
git show 8999120ff --stat 2>/dev/null | head -30 || echo "Cannot retrieve commit"
Length of output: 376
🏁 Script executed:
#!/bin/bash
# Check if model.Meta.indexes existed before migration 0111 was created
git show 8999120ff:apps/api/plane/db/models/notification.py 2>/dev/null | grep -A 30 "class Meta:" | head -40 || echo "Cannot retrieve file"Length of output: 1617
🏁 Script executed:
#!/bin/bash
# Get more context on queries that use receiver AND workspace together
rg -nP 'Notification\.objects\.(filter|exclude)' -A2 -B1 --type=py -g '!migrations' | grep -A3 'receiver\|workspace'Length of output: 1347
Remove migration 0111 indexes that duplicate model Meta.indexes definitions.
The five notification indexes added in migration 0111 are already defined in the model's Meta.indexes (apps/api/plane/db/models/notification.py, lines 39-55). Indexes defined in Meta.indexes are automatically created during migration, so redefining them in migration 0111 will cause duplicate key errors.
Either:
- Remove the model Meta.indexes definitions and keep only migration 0111, OR
- Remove migration 0111 and keep only model Meta.indexes (recommended—cleaner and more maintainable)
Additionally, verify the field order ['receiver', 'workspace', 'read_at', ...] aligns with query patterns. Queries filter on workspace__slug and receiver_id together, so consider whether starting with workspace would be more optimal than starting with receiver.
🤖 Prompt for AI Agents
In
apps/api/plane/db/migrations/0111_notification_notif_receiver_status_idx_and_more.py
around lines 14 to 34, the five notification indexes added by this migration
duplicate indexes already declared on the Notification model's Meta.indexes,
causing duplicate-key migration failures; remove these index additions from
migration 0111 (recommended) so the model Meta.indexes alone create them, or
alternatively delete the Meta.indexes and keep this migration (not recommended);
after removing the duplicates, verify and adjust the indexed field order to
match actual query patterns (queries filter on workspace__slug and receiver_id
together—consider placing 'workspace' before 'receiver' in the index definitions
if appropriate).
| AddIndexConcurrently( | ||
| model_name='fileasset', | ||
| index=models.Index(fields=['asset'], name='asset_asset_idx'), | ||
| ), |
There was a problem hiding this comment.
Undocumented index on fileasset table.
The asset_asset_idx index on the fileasset model is not mentioned in the PR objectives. Please clarify:
- Is this intentional or was the PR description incomplete?
- What query patterns does this index optimize?
Description
notificationtable to improve query performance and data access efficiency.Type of Change
References
[MOB-1199]
Note
Adds concurrent DB indexes for notifications and a new index on file assets to speed common queries.
notification:['receiver','workspace','read_at','created_at']→notif_receiver_status_idx['receiver','workspace','entity_name','read_at']→notif_receiver_entity_idx['receiver','workspace','snoozed_till','archived_at']→notif_receiver_state_idx['receiver','workspace','sender']→notif_receiver_sender_idx['workspace','entity_identifier','entity_name']→notif_entity_lookup_idxfileasset.asset→asset_asset_idx.db/models/notification.pyanddb/models/asset.pymetadata.Written by Cursor Bugbot for commit d1829d4. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.